Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Use target_link_libraries instead of ament_target_dependencies #169

Merged
merged 3 commits into from
Feb 10, 2025
Merged

Use target_link_libraries instead of ament_target_dependencies #169

merged 3 commits into from
Feb 10, 2025

Conversation

sloretz
Copy link
Contributor

@sloretz sloretz commented Feb 8, 2025

ament_target_dependencies hasn't been necessary for a while, and it can lead to libraries depending on more targets than they actually need. This PR uses target_link_libraries instead.

ament/ament_cmake#292

@ahcorde
Copy link
Contributor

ahcorde commented Feb 10, 2025

Pulls: #169
Gist: https://gist.githubusercontent.com/ahcorde/efc185ee8cd3fa86e183fa0a792b0c18/raw/70fe08543cadd856b045e0abc9311cec1401dfa7/ros2.repos
BUILD args: --packages-above-and-dependencies rmw_connextdds_common --packages-above-and-dependencies rmw_connextdds_common
TEST args: --packages-above rmw_connextdds_common --packages-above rmw_connextdds_common
ROS Distro: rolling
Job: ci_launcher
ci_launcher ran: https://ci.ros2.org/job/ci_launcher/15151

  • Linux Build Status
  • Linux-aarch64 Build Status
  • Linux-rhel Build Status
  • Windows Build Status

ahcorde
ahcorde requested changes Feb 10, 2025
View reviewed changes
@ahcorde
Update rmw_connextdds_common/CMakeLists.txt
3e4a6f4 
Signed-off-by: Alejandro Hernández Cordero <[email protected]>
@ahcorde
Copy link
Contributor

ahcorde commented Feb 10, 2025

  • Linux Build Status
  • Linux-aarch64 Build Status
  • Linux-rhel Build Status
  • Windows Build Status

@sloretz
Copy link
Contributor Author

sloretz commented Feb 10, 2025 

05:33:17 CMake Error at CMakeLists.txt:59 (target_link_libraries):
05:33:17   Target "rmw_connextdds_common_pro" links to:
05:33:17 
05:33:17     fastcdr::fastcdr
05:33:17 
05:33:17   but the target was not found.  Possible reasons include:
05:33:17 
05:33:17     * There is a typo in the target name.
05:33:17     * A find_package call is missing for an IMPORTED target.
05:33:17     * An ALIAS target is missing.
05:33:17 
05:33:17 Call Stack (most recent call first):
05:33:17   CMakeLists.txt:215 (rtirmw_add_library)

Looking into this one now

@sloretz
Fix fastcdr linking
d8a376d 
Signed-off-by: Shane Loretz <[email protected]>
@sloretz
Copy link
Contributor Author

sloretz commented Feb 10, 2025

repos build: --packages-above-and-dependencies rmw_connextdds_common --packages-above-and-dependencies rmw_connextdds_common test: --packages-above rmw_connextdds_common --packages-above rmw_connextdds_common

  • Linux Build Status
  • Linux-aarch64 Build Status
  • Linux-rhel Build Status
  • Windows Build Status

@sloretz
Copy link
Contributor Author

sloretz commented Feb 10, 2025

Aarch64 job failure is expected because CI only installs connext on amd64

09:27:41 ==> /usr/bin/colcon build --base-paths "src" --build-base "build" --install-base "install" --event-handlers console_cohesion+ console_package_list+ --cmake-args -DBUILD_TESTING=ON --no-warn-unused-cli -DINSTALL_EXAMPLES=OFF -DSECURITY=ON -DAPPEND_PROJECT_NAME_TO_INCLUDEDIR=ON --packages-above-and-dependencies rmw_connextdds_common --packages-above-and-dependencies rmw_connextdds_common
09:27:41 Package 'rmw_connextdds_common' specified with --packages-above-and-dependencies was not found

https://github.com/ros2/ci/blob/022da8a9eaa83b4ae8f7b770bdd2a4c700bf1d70/linux_docker_resources/entry_point.sh#L27-L37

fujitatomoya
fujitatomoya approved these changes Feb 10, 2025
View reviewed changes
Copy link
Collaborator

@fujitatomoya fujitatomoya left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm with green CI

@sloretz sloretz merged commit 4d1cb02 into rolling Feb 10, 2025
1 check passed
@sloretz sloretz deleted the sloretz__use_target_link_libraries branch February 10, 2025 22:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ahcorde ahcorde ahcorde requested changes

@fujitatomoya fujitatomoya fujitatomoya approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@sloretz @ahcorde @fujitatomoya